fix: review follow-ups — Arch default for at-spi fallback, input-backend degradation logging, test cleanups - #11
Merged
Merged
Conversation
…uote it in the wrapper The final fallback of _at_spi_bus_launcher returned candidates[0] (/usr/libexec, the Debian/Ubuntu/Fedora layout), which does not exist on Arch — a session where neither a candidate file nor PATH lookup finds the launcher embedded a dead path into the wrapper. The last resort is now the Arch default /usr/lib/at-spi-bus-launcher (literal, not a candidate index, so reordering candidates cannot repoint it). The resolved path is shlex.quote'd when embedded into the bash wrapper; shlex.quote leaves plain paths untouched, so behavior is unchanged on typical distros. Test updated to expect the Arch default; the wrapper test now also covers a path that requires quoting.
…wing it Both InputBackend failure sites (session_start, session_connect) caught RuntimeError and silently degraded to 'no input backend' / ydotool, so the actual reason (EIS unavailable, dbus failure, libei load error) was lost. Each site now logs a warning with the exception text before degrading; backend selection itself is unchanged. kwin_mcp has no logging module anywhere else; logging-to-stderr is the standard channel for stdio MCP servers whose stdout carries the protocol.
…ints - test_input_eis_error: dropped a dead _client_with_bus(object()) assignment immediately overwritten by _client_with_bus(_OkBus()); no side effects (pure constructor stub). - test_screenshot_fallback: 'assert Image' was always true; replaced with a real assertion that phase 2 never calls Image.frombytes for the skipped empty frame. - Type hints on fakes/helpers in both files (monkeypatch -> pytest. MonkeyPatch, tmp_path -> Path, fake signatures), per CONTRIBUTING style rules. Pre-existing tests outside the PR #10 diff untouched.
📝 Docs & SEO ReviewSource files changed in this PR: Consistency check results:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Контекст: PR #10 (порт 3 фиксов из upstream isac322#42, релиз v0.8.1). Верификация выявила 2 minor + 3 nit + типовые замечания. Этот PR исправляет ровно перечисленные находки, ничего сверх.
Находки верификации и исправления
1. session.py — финальный фолбэк резолвера at-spi-bus-launcher (minor)
Находка: последний фолбэк
_at_spi_bus_launcher()возвращал_AT_SPI_LAUNCHER_CANDIDATES[0](/usr/libexec/..., Debian/Ubuntu/Fedora-путь), а спека issue #8 требует «затем дефолт Arch». На Arch такой фолбэк — мёртвый путь в bash-обёртке.Исправлено: фолбэк — литерал
_AT_SPI_LAUNCHER_FALLBACK = "/usr/lib/at-spi-bus-launcher"(литерал вместоcandidates[1]: индекс молча переплыл бы при реордеринге кандидатов — комментарий в коде). Тестtest_at_spi_launcher_falls_back_to_whichтеперь ожидает Arch-дефолт (раньше фиксировал неверное поведение — assert на/nonexistent/a).Мутационная проверка: временно вернул
candidates[0]— тест упал; вернул — зелёный.2. core.py — причина деградации input backend глоталась молча (minor)
Находка: в
session_startиsession_connectexcept RuntimeError:без лога — «почему нет input backend» терялось.Исправлено: в обеих точках перед деградацией
logger.warning(...)с текстом исключения. Механизм: в пакете логирования нет вовсе (grep поlogging|getLoggerпуст),tool_errorне подходит (он бросает ToolError, а деградация — не ошибка); выбран stdliblogging→ stderr, стандартный канал для stdio-MCP (stdout занят протоколом). Само поведение деградации (выбор бэкенда) не изменено.Проверка: runtime-скрипт с заглушкой
InputBackend, бросающейRuntimeError— результатsession_startпрежний ("No input backend available"), warning с текстом причины пойман хендлером.3. session.py — путь в bash-обёртку без кавычек (nit)
Находка:
{_at_spi_bus_launcher()} --launch-immediatelyинтерполировался без кавычек.Исправлено:
shlex.quote(...)(+ import). Отклонение от ожидания инструкции:shlex.quoteне добавляет кавычек путям без спецсимволов, поэтому тест оставлен на прежнюю подстроку без кавычек и дополнен вторым кейсом/opt/my tools/launcher, где кавычки реально появляются — так тест честен и покрывает кавычки.4. tests/test_input_eis_error.py — мёртвое присваивание (nit)
Находка:
client = _client_with_bus(object())немедленно перетирался.Исправлено: строка удалена (проверено:
_client_with_bus— чистая заглушка конструктора, без side effects).5. tests/test_screenshot_fallback.py — таутология (nit)
Находка:
assert Imageвсегда истинен.Исправлено: тест проверял реальное поведение (пропуск пустых фреймов), поэтому ассерт заменён на содержательный: phase 2 не вызывает
Image.frombytesдля пропущенного пустого фрейма (monkeypatch-счётчик,saved == []). Соседние сценарии не тронуты, тест-счётчик не уменьшился.6. Type hints в тестах #10 (typo-level)
Находка: CONTRIBUTING требует хинты на всех сигнатурах; новые хелперы из PR #10 частично без хинтов.
Исправлено:
fake_dbus/failing_dbus(address: str, path: Path -> Path),fake_spectacle/failing_spectacle(output_path: Path -> None),fake_raw_frame,monkeypatch: pytest.MonkeyPatch,tmp_path: Pathв обоих файлах + сигнатураtest_setup_translates_connect_failure_from_interface_proxy. Фикстуры-параметры в test_session_startup.py не хинтил — против конвенции файла (соседние тесты их не хинтят). Пре-экзистинг тесты вне диффа #10 не тронуты.Релиз v0.8.2
CHANGELOG (2 user-facing fixed + 1 internal), pyproject 0.8.2,
uv lock,scripts/sync_plugin_version.py(3 манифеста, --check зелёный), README v0.8.1 → v0.8.2.Проверка
uv run ruff check src/ tests/— All checks passeduv run ruff format --check .— 40 files already formatteduv run ty check src/— All checks passeduv run pytest tests/ -q— 65 passed (до: 65; тауологичный тест не удалён — заменён ассерт)